Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'Solution' #526

Closed
wants to merge 1 commit into from
Closed

'Solution' #526

wants to merge 1 commit into from

Conversation

NazarSlavych
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your hard work on the Battleship game implementation! 🚢 While your code meets most of the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:

  1. Ship Configuration Validation: The _validate_field method needs to ensure that ships are not placed in neighboring cells, including diagonally. This is crucial for the correct functionality of the game.

  2. Ship Lengths and Counts: Make sure that the number of ships and their lengths align with the task requirements. The condition len(ship_lengths) != 10 might not be correct if the task specifies a different configuration.

  3. Import Statement: Please correct the import statement to use commas instead of a forward slash: from typing import List, Tuple, Dict, Optional.

Once these issues are resolved, your code will be in great shape! Keep up the great work, and I'm looking forward to seeing the improvements. 😊

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

@@ -1,34 +1,111 @@
from typing import List, Tuple, Dict, Optional

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import statement is incorrect due to the use of a forward slash instead of a comma. It should be from typing import List, Tuple, Dict, Optional.

Comment on lines +86 to +87
if len(ship_lengths) != 10:
raise ValueError("There must be exactly 10 ships.")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition len(ship_lengths) != 10 might not be correct if the task requires a specific number of ships with specific lengths. Ensure that the number of ships and their lengths match the task requirements.

Comment on lines +101 to +103
if (counts[1] != 4 or counts[2] != 3
or counts[3] != 2 or counts[4] != 1):
raise ValueError("Invalid ship configuration.")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation logic for ship configuration assumes a specific distribution of ship lengths. Ensure this matches the task requirements. If the task specifies a different configuration, adjust the counts accordingly.

@NazarSlavych NazarSlavych deleted the develop branch January 5, 2025 20:30
@NazarSlavych NazarSlavych restored the develop branch January 5, 2025 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants